-
Notifications
You must be signed in to change notification settings - Fork 7
Minimal validator #1187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Minimal validator #1187
Conversation
Frostman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fredi-raspall, we need to agree on the output format; the current one would be very difficult to consume to produce a user-friendly message and difficult to parse on top of it. I'd suggest to at least flatten it a bit into list of errors and move the error type into field type instead of it being a key and always have an object as a root, e.g.:
{
"success": true
}
or
{
"errors": [
{
"type": "DeserializeError",
"message": "invalid value: integer `-1500`, expected u32 at line 79 column 20",
"line": 79,
"column": 20,
"category": "Data",
"context": {
"74": " },",
"75": " \"protocolIP\": \"7.0.0.100/32\",",
"76": " \"vtepIP\": \"7.0.0.100/32\",",
"77": " \"vtepMAC\": \"02:aa:bb:cc:dd:ee\",",
"78": " \"vtepMTU\": -1500,",
"79": " \"workers\": 1",
"80": " },",
"81": " \"groups\": {",
"82": " \"gw-group-1\": {",
"83": " \"members\": ["
}
},
{
"type": "ConversionError",
"message": "Invalid Gateway Agent object: Could not create VPC: '0' is not a valid VNI"
},
{
"type": "ConfigurationError",
"message": "blah blah 1"
},
{
"type": "ConfigurationError",
"message": "blah blah 2"
},
{
"type": "ConfigurationError",
"message": "blah blah 3"
},
]
}
I additionally suggest renaming hint to message and making it mandatory for all errors to have a message.
validator/Cargo.toml
Outdated
| ordermap = { workspace = true, features = ["serde"] } | ||
|
|
||
| serde = { workspace = true } | ||
| serde_json = { workspace = true } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newline missing
Ok, I'll see what I can do. Note that some of the errors are exclusive. I.e., if we fail to deserialize, we'll not be able to validate the config. So you'll never see "DeserializeError" along with "ConversionError".
Ok! |
5ec6a34 to
b4cdc48
Compare
|
@Frostman , I've pushed a new version with a simplified output. Samples I wonder if we should add an optional "rule" field (or "rule-violation") that explains the user the reason why a configuration may be rejected in general terms. E.g. |
5fd28e6 to
799b546
Compare
|
Tried the latest binary @Fredi-raspall shared in Slack, and it looks ok. I think we can merge in that state and main work would be to make all validation messages as user friendly as possible. |
I will squash the 3 commits first. |
8c4d0fa to
f0ec3a1
Compare
We need a new crate to be able to validate gw configurations offline,
without starting dataplane. This patch adds a vanilla validator,
as a rust binary. The intent is to compile this validator as wasm/wasi
and use it externally. The validator expects a GatewayAgent CRD from
stdin in JSON/YAML and produces a YAML-encoded response in stdout.
If the response cannot be serialized, an error is written to stderr.
On success, the validator outputs:
success: true
errors: []
In case the configuration is invalid, or cannot be validated for
some reason (e.g. malformed json/yaml), a yaml-encoded string is
output to stderr, with success = false and the errors populated.
E.g.
success: false
errors:
- type: Conversion
message: 'Invalid Gateway Agent object: Could not create VPC: ''0''
is not a valid VNI'
or
success: false
errors:
- type: Deserialization
message: 'spec.gateway.neighbors[1].asn: invalid value:
integer `6500200000000`, expected u32 at line 68 column 16'
Each error includes a type and a text message describing the problem.
The possible error types mirror the sequence of steps the validator
takes to evaluate a configuration and is one of:
"Environment": for errors of the tool
"Deserialization": for malformed inputs or invalid types
"Metadata": for illegal or missing metadata fields
"Conversion": for errors converting CRD to configuration
"Configuration": for missing or semantically incorrect configs
In general, only the last two types of errors may be due to an
incorrect configuration from the user. E.g.
success: false
errors:
- type: Configuration
message: A VPC peering object refers to non-existent VPC 'VPC-1'
Signed-off-by: Fredi Raspall <[email protected]>
f0ec3a1 to
1186057
Compare
Addresses #1174
I will open a separate PR to:
* refine the feedback and the information provided by some of the errors.
* move some checks from conversion to validation, as they pertain there.
* see if an exhaustive (non-early-exited) validation can be added.
Sample deserialization errors
In these errors, a list of lines (context) near the issue is returned. The line number is not too exact (depending on the error type it is offset by +/-1 or 2). We should seldom (if ever) see deserialization errors when the input JSON is machine generated. So this is mostly for testing manually crafted configs.
Sample Metadata errors
Sample Conversion errors
Sample Configuration errors
Success case